Conversation
|
Thanks for the contribution! 💯 On first pass this looks good. I have some follow up questions
|
|
Testing this is an interesting case. Since this is only modifying the Docker Container Create SDK call, I think we just need to test that the container.create SDK call is kicked off with the expected parameter. It is equivalent to including the 'tty=False' in the container create call. To test these today, we are verifying the Container Create call was kicked off with the correct parameters passed. The proposed PR maintains that standard of testing with the underlying Docker SDKs by asserting docker.create passed in the use_config_proxy=True parameter. Any issues/disagreement with that train of thought? |
|
@cooper3330 Your testing rationale makes sense. I digged thru Docker-Py's recent commit of docker/docker-py#2202 to understand the implementation. I am confident that this doesn't have any side effects until you set proxy config in global Docker configuration files. But this feature is available in Docker SDK >= 3.7.0. We need to update the version in |
To support Docker Proxy Config (aws#1196)
|
Bumping docker version separately - #1214 so we know this is safe before merging this. |
Issue #, if available:
#913
Description of changes:
This updates the docker create command to configure the proxy settings inside the docker container using global docker configuration file. This will allow users who sit behind a proxy and have configured their proxy settings in docker to pass through their proxy for things such as dependency installation (Pip, etc.).
Checklist:
make prpassesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.